Skip to content

Conversation

@johnsimeroth
Copy link

Description

This PR implements the uploadBytes storage function for the modular SDK, one of several unimplemented methods mentioned in #7483.

Related issues

#7483

Release Summary

Adds missing storage.uploadBytes implementation

Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
    • Yes
  • My change supports the following platforms;
    • Android
    • iOS
    • Other (macOS, web)
  • My change includes tests;
    • e2e tests added or updated in packages/\*\*/e2e
    • jest tests added or updated in packages/\*\*/__tests__
  • I have updated TypeScript types that are affected by my change. (N/A)
  • This is a breaking change;
    • Yes
    • No

Test Plan

No new tests, but all existing storage tests still pass. I would have mirrored any tests for uploadBytesResumable, but I didn't see any for that either.

image

Think react-native-firebase is great? Please consider supporting the project with any of the below:

🔥 My first PR here, LMK what you'd like to see different and I'm happy to make changes.

@vercel
Copy link

vercel bot commented Sep 13, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
react-native-firebase Ready Ready Preview Comment Sep 13, 2025 6:13pm

@CLAassistant
Copy link

CLAassistant commented Sep 13, 2025

CLA assistant check
All committers have signed the CLA.

@MichaelVerdon
Copy link
Collaborator

Can you write a test for this please?

@github-actions
Copy link

Hello 👋, this PR has been opened for more than 2 months with no activity on it.

If you think this is a mistake please comment and ping a maintainer to get this merged ASAP! Thanks for contributing!

You have 15 days until this gets closed automatically

@github-actions github-actions bot added the Stale label Oct 14, 2025
@github-actions github-actions bot closed this Oct 29, 2025
@mikehardy mikehardy reopened this Oct 29, 2025
@github-actions github-actions bot removed the Stale label Oct 29, 2025
@github-actions
Copy link

Hello 👋, this PR has been opened for more than 2 months with no activity on it.

If you think this is a mistake please comment and ping a maintainer to get this merged ASAP! Thanks for contributing!

You have 15 days until this gets closed automatically

@github-actions github-actions bot added the Stale label Nov 26, 2025
@mikehardy
Copy link
Collaborator

It is nearly impossible to write automated tests for these storage upload/download methods because they run against an in-memory emulator on same host as the client, so there is insufficient delay (even with very large files) for a test to work.

I believe the only valid tests would work against cloud storage vs the emulator - which is possible now.

This is the entry point for our local tests:

https://github.com/invertase/react-native-firebase/blob/main/tests/local-tests/index.js

And if you run yarn tests:android:manual or yarn tests:ios:manual then use the little top menu of manual tests in the app once it starts, you can exercise something difficult to automate. If you do not put an emulator call in there it will go against the cloud storage, so in that way you could easily do a little test React Component that uploaded a file (then downloaded it and compared it I suppose) to make sure it was working.

Copy link
Collaborator

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code seems reasonable - I assume this is working for you @johnsimeroth ?

@mikehardy
Copy link
Collaborator

None of the CI errors are related to this PR - if you rebase to current main in this repo there are a bunch of CI fixes that should see things go green

@github-actions github-actions bot removed the Stale label Nov 29, 2025
@vercel
Copy link

vercel bot commented Dec 4, 2025

@johnsimeroth is attempting to deploy a commit to the Invertase Team on Vercel.

A member of the Team first needs to authorize it.

@johnsimeroth
Copy link
Author

@mikehardy

The code seems reasonable - I assume this is working for you @johnsimeroth ?

Apologies for the delay! Looks like I need to sort out my github notification settings, I wasn't getting any of the responses here and I honestly forgot about this PR.

Yes, the code is the same that I added to my app, and it's been working as expected without issue.

It is nearly impossible to write automated tests for these storage upload/download methods because they run against an in-memory emulator on same host as the client, so there is insufficient delay (even with very large files) for a test to work.

I believe the only valid tests would work against cloud storage vs the emulator - which is possible now.

This is the entry point for our local tests:

https://github.com/invertase/react-native-firebase/blob/main/tests/local-tests/index.js

And if you run yarn tests:android:manual or yarn tests:ios:manual then use the little top menu of manual tests in the app once it starts, you can exercise something difficult to automate. If you do not put an emulator call in there it will go against the cloud storage, so in that way you could easily do a little test React Component that uploaded a file (then downloaded it and compared it I suppose) to make sure it was working.

That seems sensible - If you'd like to see that setup as part of this PR I can find some time next week to take a crack at it.

None of the CI errors are related to this PR - if you rebase to current main in this repo there are a bunch of CI fixes that should see things go green

In the interim, I've rebased as requested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants